-
Notifications
You must be signed in to change notification settings - Fork 79
1084 db changes #1201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
1084 db changes #1201
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
point -> points
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
Thanks @antgonza! Comments addressed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to confirm, there's no possible way we would get to this line if there are no studies right? i.e. is there any way that studies could be an empty list? If so, then min would error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, there will be at least one study.
|
👍, very minor comments. |
|
Thanks @ElDeveloper, I've addressed the comments. |
|
Thanks @josenavas, 👍 On (May-25-15|15:06), josenavas wrote:
|
Tests are expected to fail. Note that this is going to the branch for fixing 1084.
Makes the needed DB changes.
The DB changes in the patch could be done in SQL. However, before I can execute those changes I need to perform some DB and filesystem cleaning (i.e. moving files to the upload folder again), so that's why those changes are performed in the python patch.